Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go: add zstyle to prevent listing packages from GOROOT and GOPATH #750

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Jun 3, 2020

Adding all of GOROOT means all of stdlib is added as completions to many
commands, which I find rather noisy and rarely useful.

Before:

[~/check]% go install <Tab>
archive/    context/    fmt/        log/        regexp/     testing/
bufio/      crypto/     go/         math/       runtime/    text/
builtin/    database/   hash/       mime/       sort/       time/
bytes/      debug/      html/       net/        strconv/    unicode/
check.go    encoding/   image/      os/         strings/    unsafe/
cmd/        errors/     index/      path/       sync/       vendor/
compress/   expvar/     internal/   plugin/     syscall/
container/  flag/       io/         reflect/    testdata/

After:

[~/check]% go install <Tab>
# completes to ./check.go, which is the only file in this small package.

Also add a setting to disable GOPATH, as it's kind deprecated and on its
way out. Sometimes I have some stuff "go get"'d in there, but I rarely
want that as completions.

Adding all of GOROOT means all of stdlib is added as completions to many
commands, which I find rather noisy and rarely useful.

Before:

    [~/check]% go install <Tab>
    archive/    context/    fmt/        log/        regexp/     testing/
    bufio/      crypto/     go/         math/       runtime/    text/
    builtin/    database/   hash/       mime/       sort/       time/
    bytes/      debug/      html/       net/        strconv/    unicode/
    check.go    encoding/   image/      os/         strings/    unsafe/
    cmd/        errors/     index/      path/       sync/       vendor/
    compress/   expvar/     internal/   plugin/     syscall/
    container/  flag/       io/         reflect/    testdata/

After:

    [~/check]% go install <Tab>
    # completes to ./check.go, which is the only file in this small package.

Also add a setting to disable GOPATH, as it's kind deprecated and on its
way out. Sometimes I have some stuff "go get"'d in there, but I rarely
want that as completions.
@arp242
Copy link
Contributor Author

arp242 commented Jun 3, 2020

FYI @pseyfert, since you made a number of changes to the Go completion recently.

@arp242
Copy link
Contributor Author

arp242 commented Jun 3, 2020

btw, IMHO the no-goroot would be better inverted as add-goroot, since I think not adding it by default makes more sense, but that's a change in the default behaviour. I don't know what this project's policy is on that – and perhaps some other people do like having it – so I opted for the smallest change.

@okapia
Copy link
Contributor

okapia commented Jun 4, 2020

Rather than have a custom style, all calling of external commands is supposed to be wrapped by the standard _call_program function. This provides the same functionality in a consistent way across all zsh completions.

So it should just be something like:

gopaths=("${(s/:/)$(_call_program go-packages go env GOPATH)}")

Then you can use the command style. Which is also more flexible because in addition to disabling it, you can change the command to something else.

@pseyfert
Copy link
Contributor

pseyfert commented Jun 4, 2020

Thanks for highlighting me.
Background for GOROOT I believe I had go build/run in mind and then wanted the completion to provide anything that would result in a valid call of the go tool and find it often frustrating when completions do pre-selections for me of what valid calls are deemed worthy to be suggested (now I'm wondering why one would recompile standard libraries or how many executables are in the standard lib).
Admittedly now that you mention it, for practical work GOROOT adds more noise than anything useful, so disabling completion of GOROOT by default and re-enable by default sounds good to me. The maintainers can share their point of view, i would say that this change of behaviour is an improvement in usability.

For GOPATH, while not needed/used with modules, my filter bubble still likes/uses it (not exclusively that is). Having a style to en/disable if GOPATH gets suggested seems a way to fit different preferences. For the default, since I use go run <TAB> for GOPATH (and go run ./<TAB> for local files) I'm probably biassed towards keeping GOPATH completion on by default. Shall we say while GO111MODULE defaults to auto (the case for go 1.14 iiuc), we leave GOPATH completion on by default and when GO111MODULE defaults to off in a future go release, we update the completion default?

@pseyfert
Copy link
Contributor

pseyfert commented Jun 17, 2020

I took the liberty to implement @okapia 's suggestion in pseyfert@18de228 .
What I don't see is how to implement disable-by-default with _call_program though.

(I was thinking that the behavior of completing GOPATH or not could also be made "smart" by checking how GO111MODULE is set - though if it's on auto one would need to check if there's a go.mod file around or call go mod and check the return code … so that would come with some complexity)

@arp242
Copy link
Contributor Author

arp242 commented Jun 17, 2020

Ah cheers, I looked at it and got confused and decided I need to sit down and read up a bit more on how this whole thing works, but haven't had the time yet 😅

@nicoulaj
Copy link
Member

Did you guys reach a consensus on this ? Personally I'm fine with @arp242's PR since it looks easier for users, but I am not familiar enough with go to make a decision.

@arp242
Copy link
Contributor Author

arp242 commented Dec 28, 2020

This has been sitting in inbox to look at; I've been pretty burned out by programming etc. in the last few months, so I haven't had the energy 😅 But feeling better now, and finishing up loads of stuff like this. I'll look again in a few days after I finish some other things and get back to ya'll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants